Skip to content

Conversation

fmease
Copy link
Member

@fmease fmease commented Feb 20, 2024

  1. commit: refactor the AST of type alias where clauses
    • I could no longer bear the look of .0.1 and .1.0
    • Arguably moving split out of TyAlias into a substruct might not make that much sense from a semantic standpoint since it reprs an index into TyAlias.predicates but it's alright and it cleans up the usage sites of TyAlias
  2. commit: fix an oversight: An empty leading where clause is still a leading where clause
    • semantically reject empty leading where clauses on lazy type aliases
      • e.g., on #![feature(lazy_type_alias)] type X where = ();
    • make empty leading where clauses on assoc types trigger lint deprecated_where_clause_location
      • e.g., impl Trait for () { type X where = (); }

@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2024

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 20, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2024

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

#[suggestion_part(code = "{snippet}")]
pub right: Span,

pub enum WhereClauseBeforeTypeAliasSugg {
Copy link
Member Author

@fmease fmease Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The split into Remove & Move was not only done to provide a more accurate diagnostic message for the edge case “two where clauses, a leading and a trailing one, where the leading one is empty” but also to prevent the debug assertion empty span and no suggestions from firing.

db.note(
"see issue #89122 <https://github.com/rust-lang/rust/issues/89122> for more information",
);
BuiltinLintDiagnostics::DeprecatedWhereclauseLocation(sugg) => {
Copy link
Member Author

@fmease fmease Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's sad that I couldn't consolidate the messages of the lint and the error but the bridging between rustc_ast_passes & rustc_lint is too tricky. If rustc_lint_defs (the go-between) had a messages.ftl I could make both crates import the fluent_generated messages but that's currently not the case.

@fmease fmease changed the title Detect empty leading where clauses on type aliases Reject empty leading where clauses on type aliases Feb 20, 2024
@fmease fmease changed the title Reject empty leading where clauses on type aliases Detect empty leading where clauses on type aliases Feb 20, 2024
@fmease fmease force-pushed the detect-empty-leading-where-clauses-on-ty-aliases branch from adf7ddc to dbd70e5 Compare February 20, 2024 06:17
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for a thoughtful and well commented pr as always

@compiler-errors
Copy link
Member

sorry for taking so long to review xd

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 28, 2024

📌 Commit dbd70e5 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 28, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 28, 2024
…clauses-on-ty-aliases, r=compiler-errors

Detect empty leading where clauses on type aliases

1. commit: refactor the AST of type alias where clauses
   * I could no longer bear the look of `.0.1` and `.1.0`
   * Arguably moving `split` out of `TyAlias` into a substruct might not make that much sense from a semantic standpoint since it reprs an index into `TyAlias.predicates` but it's alright and it cleans up the usage sites of `TyAlias`
2. commit: fix an oversight: An empty leading where clause is still a leading where clause
   * semantically reject empty leading where clauses on lazy type aliases
     * e.g., on `#![feature(lazy_type_alias)] type X where = ();`
   * make empty leading where clauses on assoc types trigger lint `deprecated_where_clause_location`
     * e.g., `impl Trait for () { type X where = (); }`
@bors
Copy link
Collaborator

bors commented Feb 28, 2024

☔ The latest upstream changes (presumably #121489) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 28, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 29, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#110543 (Make `ReentrantLock` public)
 - rust-lang#121689 ([rustdoc] Prevent inclusion of whitespace character after macro_rules ident)
 - rust-lang#121724 (Use `LitKind::Err` for malformed floats)
 - rust-lang#121735 (pattern analysis: Don't panic when encountering unexpected constructor)
 - rust-lang#121743 (Opportunistically resolve regions when processing region outlives obligations)

Failed merges:

 - rust-lang#121326 (Detect empty leading where clauses on type aliases)
 - rust-lang#121416 (Improve error messages for generics with default parameters)
 - rust-lang#121669 (Count stashed errors again)
 - rust-lang#121723 (Two diagnostic things)

r? `@ghost`
`@rustbot` modify labels: rollup
@fmease fmease force-pushed the detect-empty-leading-where-clauses-on-ty-aliases branch from dbd70e5 to cce8128 Compare February 29, 2024 16:20
@fmease
Copy link
Member Author

fmease commented Feb 29, 2024

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Feb 29, 2024

📌 Commit cce8128 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 29, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 29, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121326 (Detect empty leading where clauses on type aliases)
 - rust-lang#121464 (rustc: Fix wasm64 metadata object files)
 - rust-lang#121681 (Safe Transmute: Revise safety analysis)
 - rust-lang#121753 (Add proper cfg to keep only one AlignmentEnum definition for different target_pointer_widths)
 - rust-lang#121782 (allow statics pointing to mutable statics)
 - rust-lang#121798 (Fix links in rustc doc)
 - rust-lang#121806 (add const test for ptr::metadata)
 - rust-lang#121809 (Remove doc aliases to PATH)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dd4ecd1 into rust-lang:master Mar 1, 2024
@rustbot rustbot added this to the 1.78.0 milestone Mar 1, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2024
Rollup merge of rust-lang#121326 - fmease:detect-empty-leading-where-clauses-on-ty-aliases, r=compiler-errors

Detect empty leading where clauses on type aliases

1. commit: refactor the AST of type alias where clauses
   * I could no longer bear the look of `.0.1` and `.1.0`
   * Arguably moving `split` out of `TyAlias` into a substruct might not make that much sense from a semantic standpoint since it reprs an index into `TyAlias.predicates` but it's alright and it cleans up the usage sites of `TyAlias`
2. commit: fix an oversight: An empty leading where clause is still a leading where clause
   * semantically reject empty leading where clauses on lazy type aliases
     * e.g., on `#![feature(lazy_type_alias)] type X where = ();`
   * make empty leading where clauses on assoc types trigger lint `deprecated_where_clause_location`
     * e.g., `impl Trait for () { type X where = (); }`
@fmease fmease deleted the detect-empty-leading-where-clauses-on-ty-aliases branch March 1, 2024 00:52
calebcartwright pushed a commit to calebcartwright/rust that referenced this pull request Jun 22, 2024
…clauses-on-ty-aliases, r=compiler-errors

Detect empty leading where clauses on type aliases

1. commit: refactor the AST of type alias where clauses
   * I could no longer bear the look of `.0.1` and `.1.0`
   * Arguably moving `split` out of `TyAlias` into a substruct might not make that much sense from a semantic standpoint since it reprs an index into `TyAlias.predicates` but it's alright and it cleans up the usage sites of `TyAlias`
2. commit: fix an oversight: An empty leading where clause is still a leading where clause
   * semantically reject empty leading where clauses on lazy type aliases
     * e.g., on `#![feature(lazy_type_alias)] type X where = ();`
   * make empty leading where clauses on assoc types trigger lint `deprecated_where_clause_location`
     * e.g., `impl Trait for () { type X where = (); }`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants